Skip to content

NMT-1521: Add per interface ipv4 forwarding test. - #1372

Merged
kubevirt-bot merged 1 commit into
nmstate:mainfrom
emy:NMT-1521-e2e-upstream
Dec 1, 2025
Merged

kubevirt-bot merged 1 commit into
nmstate:mainfrom
emy:NMT-1521-e2e-upstream

Conversation

@emy

@emy emy commented Sep 22, 2025 •

Copy link
Copy Markdown
Member

Is this a BUG FIX or a FEATURE ?:

Uncomment only one, leave it on its own line:

/kind bug

/kind enhancement

What this PR does / why we need it:
As discussed with @qinqon we're introducing an example test for per interface ipv4 forwarding.

Special notes for your reviewer:

Release note:

NONE

Summary by CodeRabbit

  • Documentation
    • Added a new example manifest demonstrating per-interface IPv4 forwarding using a NodeNetworkConfigurationPolicy. The example targets interface eth1, sets it up, enables IPv4 with DHCP, and turns on forwarding.
  • Tests
    • Expanded example coverage by adding a corresponding test entry for the new per-interface IPv4 forwarding example.

Signed-off-by: Emilia Desch <emy@redhat.com>
@kubevirt-bot kubevirt-bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Sep 22, 2025
@coderabbitai

coderabbitai Bot commented Sep 22, 2025 •

Copy link
Copy Markdown

Walkthrough

Adds a new example manifest for a NodeNetworkConfigurationPolicy enabling IPv4 forwarding on eth1, and registers this example in the test examples list. No other files or behaviors are modified.

Changes

Cohort / File(s) Summary
New example manifest
docs/examples/per-interface-ipv4-forwarding.yaml
Adds a NodeNetworkConfigurationPolicy example named per-interface-ipv4-forwarding configuring eth1 (ethernet, up) with IPv4 DHCP enabled and forwarding true.
Test examples registry update
test/doc/examples.go
Adds an ExampleSpec entry: name "Per interface IPv4 forwarding", file "per-interface-ipv4-forwarding.yaml", policy "per-interface-ipv4-forwarding", ifaceNames ["eth1"].

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I hop through YAML fields so bright,
eth1 wakes, its flags alight—
DHCP whispers, routes take wing,
Forward, packets! Spring to spring.
In tests we note this tidy feat,
A carrot checkmark: crisp and neat. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "NMT-1521: Add per interface ipv4 forwarding test." clearly and concisely summarizes the primary change—adding a per‑interface IPv4 forwarding test/example—and includes the tracking ticket; this matches the added example manifest and test registration in the PR.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c85fc6e and c656ecb.

📒 Files selected for processing (2)
  • docs/examples/per-interface-ipv4-forwarding.yaml (1 hunks)
  • test/doc/examples.go (1 hunks)

Comment thread test/doc/examples.go
@nmstate nmstate deleted a comment from coderabbitai Bot Sep 24, 2025
@qinqon

qinqon commented Sep 24, 2025

Copy link
Copy Markdown
Member

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new example and a corresponding test for configuring per-interface IPv4 forwarding. The changes are clear and well-contained. My review includes one suggestion for the new example YAML file to improve its conciseness by removing a redundant property, which will help align it with best practices for users who might copy this example.

Comment thread docs/examples/per-interface-ipv4-forwarding.yaml
@qinqon

qinqon commented Sep 26, 2025

Copy link
Copy Markdown
Member

@emy looks like like the upgrade test fail at the test ?

@mkowalski

Copy link
Copy Markdown
Member

But it's not only that failing... I see also

   Error from server (NotFound): nodenetworkconfigurationpolicies.nmstate.io "dns" not found
  [FAILED] 
  Unexpected error:
      <*errors.errorString | 0xc000698ec0>: 
      ./cluster/kubectl.shwait nncp dns --for condition=Available --timeout 3m : selecting podman as container runtime
      Error from server (NotFound): nodenetworkconfigurationpolicies.nmstate.io "dns" not found
       exit status 1
      {
          s: "./cluster/kubectl.shwait nncp dns --for condition=Available --timeout 3m : selecting podman as container runtime\nError from server (NotFound): nodenetworkconfigurationpolicies.nmstate.io \"dns\" not found\n exit status 1",
      }
  occurred 

as well as

   Error from server (NotFound): nodenetworkconfigurationpolicies.nmstate.io "linux-bridge-vlan" not found
  [FAILED] 
  Unexpected error:
      <*errors.errorString | 0xc000bbcd00>: 
      ./cluster/kubectl.shwait nncp linux-bridge-vlan --for condition=Available --timeout 3m : selecting podman as container runtime
      Error from server (NotFound): nodenetworkconfigurationpolicies.nmstate.io "linux-bridge-vlan" not found
       exit status 1
      {
          s: "./cluster/kubectl.shwait nncp linux-bridge-vlan --for condition=Available --timeout 3m : selecting podman as container runtime\nError from server (NotFound): nodenetworkconfigurationpolicies.nmstate.io \"linux-bridge-vlan\" not found\n exit status 1",
      } 

@mkowalski

Copy link
Copy Markdown
Member

/retest

1 similar comment
@mkowalski

Copy link
Copy Markdown
Member

/retest

@qinqon

qinqon commented Nov 28, 2025

Copy link
Copy Markdown
Member

/lgtm
/approve

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 28, 2025
@kubevirt-bot

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qinqon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 28, 2025
@emy

emy commented Dec 1, 2025

Copy link
Copy Markdown
Member Author

/retest-required

@kubevirt-bot
kubevirt-bot merged commit 735e04e into nmstate:main Dec 1, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants